Optimize leastActiveSelect and weight test case #2172

carryxyh · 2018-08-02T11:17:33Z

Now the select is select a random one when there are several least active invokers and all of them are in warm up.
After this pr, it will select also by weight and warm up.

And fix a bug when two invoker's active is same and weight not same.

issue:
#904

codecov-io · 2018-08-02T11:47:05Z

Codecov Report

Merging #2172 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #2172      +/-   ##
============================================
- Coverage     55.05%   55.01%   -0.04%     
- Complexity     5282     5288       +6     
============================================
  Files           573      573              
  Lines         25490    25538      +48     
  Branches       4526     4531       +5     
============================================
+ Hits          14033    14050      +17     
- Misses         9362     9387      +25     
- Partials       2095     2101       +6

Impacted Files	Coverage Δ	Complexity Δ
...pc/cluster/loadbalance/LeastActiveLoadBalance.java	`88.23% <100%> (+17.64%)`	`9 <0> (+3)`	⬆️
...che/dubbo/remoting/transport/mina/MinaChannel.java	`42.25% <0%> (-11.27%)`	`15% <0%> (-1%)`
...org/apache/dubbo/rpc/filter/ActiveLimitFilter.java	`77.77% <0%> (-11.12%)`	`5% <0%> (-2%)`
...he/dubbo/remoting/transport/netty/NettyClient.java	`72.88% <0%> (-10.17%)`	`12% <0%> (-1%)`
...onfig/spring/extension/SpringExtensionFactory.java	`75.86% <0%> (-9.86%)`	`11% <0%> (+1%)`
...apache/dubbo/rpc/protocol/dubbo/FutureAdapter.java	`58.06% <0%> (-6.46%)`	`3% <0%> (ø)`
...ubbo/config/spring/status/SpringStatusChecker.java	`75.67% <0%> (-5.58%)`	`11% <0%> (ø)`
.../config/spring/status/DataSourceStatusChecker.java	`71.79% <0%> (-4.68%)`	`6% <0%> (ø)`
...exchange/support/header/HeaderExchangeChannel.java	`79.16% <0%> (-4.35%)`	`35% <0%> (ø)`
...ubbo/rpc/protocol/dubbo/ChannelWrappedInvoker.java	`37.5% <0%> (-4.17%)`	`3% <0%> (ø)`
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b411a77...cd05619. Read the comment docs.

imsunv · 2018-08-11T11:49:31Z

@carryxyh

I saw your pr, I think the start warm up feature with the leastactive algorithm for loadbalance will be conflicted, because of the active for the new provider will be the least.

So is it necessary to calculate the weight for warm up?

Or maybe we can consider another implementation to supports warmup for leastactive loadbalance.

carryxyh · 2018-08-13T04:10:45Z

This is not the problem that this PR has to solve. The problem that this PR has to solve is too many loops and a small bug.
And I think the problem you are talking about is not a big problem. Just need to prompt the user in the document: If the cluster uses LeastActive as the load balancing policy, then if the new service is in the warm-up period, then it will get more requests (because the new service itself is the least active).

carryxyh · 2018-08-13T04:14:25Z

So is it necessary to calculate the weight for warm up?
Of course.
If all the service is in warm-up. Or two least active service is in warm-up, if not calc weight after warm-up, u will get a wrong weight.

diecui1202 · 2018-08-16T09:52:29Z

...o-cluster/src/main/java/org/apache/dubbo/rpc/cluster/loadbalance/LeastActiveLoadBalance.java

@@ -41,26 +39,26 @@
        int leastActive = -1; // The least active value of all invokers
        int leastCount = 0; // The number of invokers having the same least active value (leastActive)
        int[] leastIndexs = new int[length]; // The index of invokers having the same least active value (leastActive)
-        int totalWeight = 0; // The sum of weights
-        int firstWeight = 0; // Initial value, used for comparision
+        int taotalWeightWithWarmUp = 0; // The sum of with warmup weights


carryxyh · 2018-08-17T07:28:30Z

@whanice @chickenlj @zonghaishang
Pls help to review this pr :)

carryxyh · 2018-08-17T08:17:10Z

@diecui1202
I think a same pr should be sent to branch 2.6.x.
How do u think about it?

diecui1202 · 2018-09-06T11:25:10Z

@carryxyh pls. check the conflict.

Agree with back to 2.6.x

carryxyh · 2018-09-06T11:26:53Z

Ok, fix it soon.

…tive invokers and all of them are in warm up. After this pr, it will select also by weight and warm up. And fix a bug when two invoker's active is same and weight not same.

carryxyh · 2018-09-20T07:40:25Z

@zonghaishang @diecui1202 @chickenlj @whanice
Hi, everyone.
Can you take the time to review this pr? This is a very simple bug, but it has been idle for a long time.

zonghaishang · 2018-09-21T04:13:51Z

...o-cluster/src/test/java/org/apache/dubbo/rpc/cluster/loadbalance/LeastActiveBalanceTest.java

+        System.out.println(sumInvoker2);
+    }
+
+    class MyLeastActiveLoadBalance extends AbstractLoadBalance {


Could you reuse existing code?

org.apache.dubbo.rpc.cluster.loadbalance.LeastActiveBalanceTest#testLeastActiveLoadBalance_select:

public void testLeastActiveLoadBalance_select() { ... Map<Invoker, AtomicLong> counter = getInvokeCounter(runs, LeastActiveLoadBalance.NAME); ...

Load your repaired code this way：

ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(LeastActiveLoadBalance.NAME)

zonghaishang · 2018-09-21T04:15:32Z

...o-cluster/src/main/java/org/apache/dubbo/rpc/cluster/loadbalance/LeastActiveLoadBalance.java

@@ -70,7 +68,7 @@
        }
        if (!sameWeight && totalWeight > 0) {
            // If (not every invoker has the same weight & at least one invoker's weight>0), select randomly based on totalWeight.
-            int offsetWeight = ThreadLocalRandom.current().nextInt(totalWeight);
+            int offsetWeight = ThreadLocalRandom.current().nextInt(totalWeight) + 1;


Why change ThreadLocalRandom.current().nextInt(totalWeight) to ThreadLocalRandom.current().nextInt(totalWeight) + 1 ?

Is it possible that +1 causes offset not equal or less 0?

If not change to +1, that will cause a bug.
U can look at my unit test for more detail.

carryxyh · 2018-09-26T10:43:55Z

@zonghaishang
I have optimize ut, pls check again.

code4wt · 2018-11-27T16:24:23Z

...o-cluster/src/main/java/org/apache/dubbo/rpc/cluster/loadbalance/LeastActiveLoadBalance.java

@@ -70,7 +68,7 @@
        }
        if (!sameWeight && totalWeight > 0) {
            // If (not every invoker has the same weight & at least one invoker's weight>0), select randomly based on totalWeight.
-            int offsetWeight = ThreadLocalRandom.current().nextInt(totalWeight);
+            int offsetWeight = ThreadLocalRandom.current().nextInt(totalWeight) + 1;


最近正好在看 LeastActiveLoadBalance 的源码，我觉得 +1 这个逻辑有点突兀，不知道背景的同学可能不知道为什么要+1。更合理的方式，我觉得应该按照 RandomLoadBalance 逻辑去处理。将 if (offsetWeight <= 0) 改为 if (offsetWeight < 0)，这样两者的逻辑能够统一起来。只要大家能看懂 RandomLoadBalance 的代码，那么此处的代码也一样能看懂，而不用特地去思考为什么要 +1。你觉得呢

This was referenced Aug 3, 2018

Optimize LeastActiveLoadBalance select. #2161

Closed

LeastActiveLoadBalance负载均衡 #904

Closed

carryxyh changed the title ~~Optimize leastActiveSelect~~ Optimize leastActiveSelect and weight test case Aug 3, 2018

diecui1202 reviewed Aug 16, 2018

View reviewed changes

carryxyh force-pushed the optimize-leastactive branch from b812b2a to 5ebab8b Compare September 6, 2018 11:32

carryxyh requested review from chickenlj and zonghaishang September 19, 2018 06:04

carryxyh self-assigned this Sep 19, 2018

carryxyh added 4 commits September 20, 2018 15:29

Now the select is select a random one when there are several least ac…

a42fb7f

…tive invokers and all of them are in warm up. After this pr, it will select also by weight and warm up. And fix a bug when two invoker's active is same and weight not same.

weight test for all the loadBalance.

64a4ec0

optimize typo

dfcba18

optimize unit test

a8dcb28

carryxyh force-pushed the optimize-leastactive branch from 5ebab8b to a8dcb28 Compare September 20, 2018 07:32

carryxyh mentioned this pull request Sep 20, 2018

Optimize least active load balance. #2540

Closed

zonghaishang reviewed Sep 21, 2018

View reviewed changes

Optimize unit test

37c927a

Optimize unit test

cd05619

carryxyh removed the request for review from chickenlj September 27, 2018 02:39

zonghaishang merged commit 6edfb1d into apache:master Sep 27, 2018

carryxyh deleted the optimize-leastactive branch September 28, 2018 03:20

carryxyh mentioned this pull request Sep 29, 2018

Optimize LeastActiveLoadBalance and add weight test case. #2584

Merged

code4wt reviewed Nov 27, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize leastActiveSelect and weight test case #2172

Optimize leastActiveSelect and weight test case #2172

carryxyh commented Aug 2, 2018

codecov-io commented Aug 2, 2018 •

edited

Loading

imsunv commented Aug 11, 2018

carryxyh commented Aug 13, 2018

carryxyh commented Aug 13, 2018

diecui1202 Aug 16, 2018

carryxyh commented Aug 17, 2018

carryxyh commented Aug 17, 2018

diecui1202 commented Sep 6, 2018

carryxyh commented Sep 6, 2018

carryxyh commented Sep 20, 2018

zonghaishang Sep 21, 2018

zonghaishang Sep 21, 2018

zonghaishang Sep 21, 2018

carryxyh Sep 26, 2018

carryxyh commented Sep 26, 2018

code4wt Nov 27, 2018 •

edited

Loading

Optimize leastActiveSelect and weight test case #2172

Optimize leastActiveSelect and weight test case #2172

Conversation

carryxyh commented Aug 2, 2018

codecov-io commented Aug 2, 2018 • edited Loading

Codecov Report

imsunv commented Aug 11, 2018

carryxyh commented Aug 13, 2018

carryxyh commented Aug 13, 2018

diecui1202 Aug 16, 2018

Choose a reason for hiding this comment

carryxyh commented Aug 17, 2018

carryxyh commented Aug 17, 2018

diecui1202 commented Sep 6, 2018

carryxyh commented Sep 6, 2018

carryxyh commented Sep 20, 2018

zonghaishang Sep 21, 2018

Choose a reason for hiding this comment

zonghaishang Sep 21, 2018

Choose a reason for hiding this comment

zonghaishang Sep 21, 2018

Choose a reason for hiding this comment

carryxyh Sep 26, 2018

Choose a reason for hiding this comment

carryxyh commented Sep 26, 2018

code4wt Nov 27, 2018 • edited Loading

Choose a reason for hiding this comment

codecov-io commented Aug 2, 2018 •

edited

Loading

code4wt Nov 27, 2018 •

edited

Loading